Thanks!

So what is_valid_steam_install essentially does is the same we do in Constants, line 31-36, right?

However we will always default to _POSSIBLE_STEAM_ROOTS[0], which at time of writing, is ~/.local/share/Steam.

That may sound a bit oversimplified, but I wonder
Can't we achieve the same by just initially setting _STEAM_ROOT = ''?
This way, when there is no valid Steam installation, we get the path /compatibilitytools.d/ which should definitively not exist.
If ~/.local/share/Steam is valid, it will be picked up by the for steam_root in _POSSIBLE_STEAM_ROOTS loop.

constants.py re-use the is_valid_steam_install function from steamutil.py?
I'm not sure if it's a good idea to import a function like this into the constant file, perhaps we'll just have to live with the duplication for now.

That would make the constants file a bit more clean.
I'm less worried about the "import a function like this into the constant file" as it is a bit messy anyway, but I'm more worried about circular imports. I think Python can handle them if there are no wildcard imports, but I'd still like to prevent them.

we still gracefully handle this by showing the main menu but with an empty combobox and blank list, but in future there is an opportunity for some slightly improved UX here

Yeah, might be worth adding a label. Something like No launchers detected.

So what is_valid_steam_install essentially does is the same we do in Constants, line 31-36, right?

Yup, exactly!

That may sound a bit oversimplified, but I wonder
Can't we achieve the same by just initially setting _STEAM_ROOT = ''?
This way, when there is no valid Steam installation, we get the path /compatibilitytools.d/ which should definitively not exist.

Huh, that's an excellent point. I wonder why it's set to a default in the first place, when I get home I'll try on main with this change to see if anything blows up!

I'm more worried about circular imports

I think Python can handle them too, but I wonder if it's a good idea to rely on circular imports in this way (util uses constants which in turn will import from util).

It's probably a stretch but I wonder if there's any PEP guidance on this sort of thing 🤔

Huh, that's an excellent point. I wonder why it's set to a default in the first place, when I get home I'll try on main with this change to see if anything blows up!

Hmm, I probably though an empty string could break something when I implemented that. Shouldn't be the case though.

I wonder if it's a good idea to rely on circular imports

Ideally I guess we would remove the logic from constants.py and completely do it in the util/main logic.
I wonder what would be the best way to do so. What we could do is to add all possible install locations directly to POSSIBLE_INSTALL_LOCATIONS and then let is_valid_launcher_installation() check them in available_install_directories().
That feels a bit strange as there are multiple entries for the same launcher then, but I would be fine with it. Maybe can you think of a more elegant solution?

Probably the easiest way would be to add something like the following below POSSIBLE_INSTALL_LOCATIONS = ... (maybe in a more clear way):

POSSIBLE_INSTALL_LOCATIONS += [
    {'install_dir': f'{root}/compatibilitytools.d/', 'display_name': 'Steam', 'launcher': 'steam', 'type': 'native', 'icon': 'steam', 'vdf_dir': f'{root}/config'}
    for root
    in _POSSIBLE_STEAM_ROOTS
]

EDIT: Sniped while writing this comment :-)

Yep, _STEAM_ROOT = '' resolves the problem. I tested by commenting out these lines, and then setting _STEAM_ROOT = '' on a separate line. Things worked fine at least in my tests, ProtonUp-Qt still gracefully falls back to the next available launcher (Lutris, in my case).

That feels a bit strange as there are multiple entries for the same launcher then

I wouldn't be against removing the conditional checks in constants.py and letting that get handled elsewhere. Our loop in available_install_directories essentially acts to filter "invalid" install paths from POSSIBLE_INSTALL_LOCATIONS, so I don't think there's any harm in having multiple paths at the same time for the same launcher.

My concern would be that the paths in _POSSIBLE_STEAM_ROOTS can exist at the same time. For example, on my system I have ~/.steam/root and ~/.steam/steam and these are symlinks to ~/.local/share/Steam. We could end up with duplicate entries for what amounts to the same Steam installation, but the upside would be that we could accomodate a very rare case where a user might intentionally have two or more Steam installations, i.e. one at ~/.local/share/Steam and one at ~/.steam/root (where this is not a symlink). Normally this is not possible, but some people have exotic setups with multiple Steam installations, and this approach of storing them all in POSSIBLE_INSTALL_LOCATIONS would allow us to detect these out of the box.

The problem becomes how to deal with the duplicate paths in an efficient way. I initially thought that we could use sets to remove duplicates, but you can't convert a list of dictionaries to a set.

So, one solution might be to first, store the os.path.realpath in the example you have above, where root would become os.path.realpath(root). This function will expand symlinks into their actual paths, so if ~/.steam/root is a symlink it will be expanded to wherever the symlink points to, such as ~/.local/share/Steam. Then once all paths in POSSIBLE_INSTALL_LOCATIONS are actual paths and not symlinks, we will handle filtering out duplicate paths in util#available_install_directories. In this function, we simply append the install path string to a list called available_dirs (

Essentially we would end up with something like this:

• POSSIBLE_INSTALL_LOCATIONS stores the expanded paths for all Steam installation folders, so even if symlinks pointing to the same installation exist, we will store them as their actual path. This means we could end up with multiple paths pointing to, say, ~/.local/share/Steam
  • To visualise, this might be something like this if ~/.local/share/Steam exists, and ~/.steam/steam and ~/.steam/root exist both as symlinks pointing to ~/.local/share/Steam: [{ 'install_dir': '/home/gaben/.local/share/Steam/compatibilitytools.d/' }, { 'install_dir': '/home/gaben/.local/share/Steam/compatibilitytools.d/' }, { 'install_dir': '/home/gaben/.local/share/Steam/compatibilitytools.d/' }]
• In util#available_install_directories, we have some way of then filtering out these duplicate entries. Since we only store the path and the path is a string, and since os.path.realpath should return the same path for two or more symlinks pointing to the same location, we would end up with available_dirs looking something like this: [ '/home/gaben/.local/share/Steam/compatibilitytools.d/', '/home/gaben/.local/share/Steam/compatibilitytools.d/', '/home/gaben/.local/share/Steam/compatibilitytools.d/' ].
  • The simplest way to do this might just be to change this line (

    ProtonUp-Qt/pupgui2/util.py

    Line 212 in 6cf1def

    return available_dirs

    ) to return list(set(available_dirs)). We cast our list available_dirs to a set (a data structure that can't have duplicates) which will remove the duplicate install path strings, and then we cast it back again to a list. Alternatively, we could just do if is_valid_launcher_installation(loc) and install_dir not in available_dirs, although these lines have be a little concerned that that approach may not be sufficient (

    ProtonUp-Qt/pupgui2/util.py

    Lines 210 to 211 in 6cf1def

    	if install_dir and os.path.exists(install_dir):
    	available_dirs.append(install_dir)

    ) - Having said that, we could always do both for an extra bit of "insurance" :-)
  • The set casting thing is possibly not very efficient, but even if all install paths were used, and account for a few extra ones in future if something like PlayOnLinux was added, we're probably never going to be working with more than 20 strings at most for the foreseeable. Rright now with all four Steam paths and allowing for 3 hypothetical extra for PlayOnLinux - a Flatpak, a Snap (not even sure one exists) and an AppImage/system install, we would end up with 17 paths. I don't see this being a huge performance issue :-)
  • Sets are I think more than just a data structure that doesn't allow duplicates, they're a mathsy concept that I don't know a whole lot about (they were covered at uni but it's been a couple years), but for our purposes they would just serve to remove duplicates.
  • Here are the docs specific to sets in Python: https://docs.python.org/3/tutorial/datastructures.html#sets

(maybe in a more clear way)

We could do it like this maybe:

# This part is from the snippet, untested :-)
POSSIBLE_STEAM_INSTALL_LOCATIONS = [
    {'install_dir': f'{root}/compatibilitytools.d/', 'display_name': 'Steam', 'launcher': 'steam', 'type': 'native', 'icon': 'steam', 'vdf_dir': f'{root}/config'}
    for root
    in _POSSIBLE_STEAM_ROOTS,
]

# Steam Flatpak/Snap
POSSIBLE_STEAM_INSTALL_LOCATIONS += [
    {'install_dir': '~/.var/app/com.valvesoftware.Steam/data/Steam/compatibilitytools.d/', 'display_name': 'Steam Flatpak', 'launcher': 'steam', 'type': 'flatpak', 'icon': 'steam', 'vdf_dir': '~/.var/app/com.valvesoftware.Steam/.local/share/Steam/config'},
    {'install_dir': '~/snap/steam/common/.steam/root/compatibilitytools.d/', 'display_name': 'Steam Snap', 'launcher': 'steam', 'type': 'snap', 'icon': 'steam', 'vdf_dir': '~/snap/steam/common/.steam/root/config'},
]

# This is our existing `POSSIBLE_INSTALL_LOCATIONS`, just with the Steam paths moved to the above list
POSSIBLE_LAUNCHER_INSTALL_LOCATIONS = [
    {'install_dir': '~/.local/share/lutris/runners/wine/', 'display_name': 'Lutris', 'launcher': 'lutris', 'type': 'native', 'icon': 'lutris', 'config_dir': '~/.config/lutris'},
    {'install_dir': '~/.var/app/net.lutris.Lutris/data/lutris/runners/wine/', 'display_name': 'Lutris Flatpak', 'launcher': 'lutris', 'type': 'flatpak', 'icon': 'lutris', 'config_dir': '~/.var/app/net.lutris.Lutris/config/lutris'},
    {'install_dir': '~/.config/heroic/tools/wine/', 'display_name': 'Heroic Wine', 'launcher': 'heroicwine', 'type': 'native', 'icon': 'heroic'},
    {'install_dir': '~/.config/heroic/tools/proton/', 'display_name': 'Heroic Proton', 'launcher': 'heroicproton', 'type': 'native', 'icon': 'heroic'},
    {'install_dir': '~/.var/app/com.heroicgameslauncher.hgl/config/heroic/tools/wine/', 'display_name': 'Heroic Wine Flatpak', 'launcher': 'heroicwine', 'type': 'flatpak', 'icon': 'heroic'},
    {'install_dir': '~/.var/app/com.heroicgameslauncher.hgl/config/heroic/tools/proton/', 'display_name': 'Heroic Proton Flatpak', 'launcher': 'heroicproton', 'type': 'flatpak', 'icon': 'heroic'},
    {'install_dir': '~/.local/share/bottles/runners/', 'display_name': 'Bottles', 'launcher': 'bottles', 'type': 'native', 'icon': 'com.usebottles.bottles'},
    {'install_dir': '~/.var/app/com.usebottles.bottles/data/bottles/runners/', 'display_name': 'Bottles Flatpak', 'launcher': 'bottles', 'type': 'flatpak', 'icon': 'com.usebottles.bottles'}
]

# Essentially this means we split the Steam paths and all other paths into their own lists, and then store the combined list as 'POSSIBLE_INSTALL_LOCATIONS'
# Doing it this way means we keep the Steam paths at the beginning of the list
# This approach be very marginally more readable since it means we keep the Steam comprehension bits separated, but really, I don't think it's a huge deal either way :-)
POSSIBLE_INSTALL_LOCATIONS = POSSIBLE_STEAM_INSTALL_LOCATIONS + POSSIBLE_LAUNCHER_INSTALL_LOCATIONS

This is all kind of off-the-top-of-my-head, I haven't sat down and tested this out much yet. It's just one approach I think could work.

In short, I think your idea of storing all the install paths is a fine approach, even if it means multiple paths and possibly multiple invalid/duplicate paths for the same launcher type. It could indirectly allow for the benefit of picking up multiple "Steam roots" (multiple "regular" Steam installs). But we'd probably need to handle removing duplicates, so we don't end up listing all of the existing symlinks on top of the actual Steam root. We have a couple of approaches on how we could handle this, and it means keeping the conditional "filtering" logic isolated to a utility function. But I haven't tested how much of this approach would work or what pitfalls we might encounter. We don't store multiple of the same path for the same launcher type, so I don't know if we would run into any Unforeseen Consequences:tm:. I imagine not if we handle duplicates properly in util#available_install_directories, but it would need testing to find out.

I can do some testing on this over the weekend to see how having multiple paths in POSSIBLE_INSTALL_LOCATIONS goes and how filtering out duplicates goes :-)

P.S. - If all else fails, just doing STEAM_ROOT = '' would fix the problem. But this approach of storing all paths has that very edge-case benefit of allowing us to detect multiple "regular" Steam installations, and allows us to keep is_valid_launcher_installation as it could be useful in future if we need more strict checks for other launchers.

I think the way I ended up explaining this sounds like we might be adding a lot of complexity, but my comment essentially boils down to incorporating all possible Steam install paths in POSSIBLE_INSTALL_LOCATIONS and then filtering out invalid paths using util#available_install_directories. This allows us to avoid any checks for valid installation paths in constants.py and do it all in util, avoiding the problem discussed of circular imports, and also the imo slightly cleaner approach of removing conditional logic from constants.py :-)

Changes are a bit WIP across commits, I will summarize and explain them afterwards :-)

Okay, I was planning to make a couple of other changes, but I think they're better for a separate PR.

Across d0e05ce and 6423f52, the following changes were made:

constants.py Changes:

• Removes the valid Steam installation paths logic checks
• Remove setting a _STEAM_ROOT variable altogether as it is no longer needed
• Build _POSSIBLE_STEAM_ROOTS slightly differently, using a list comprehension to join the paths with our HOME_DIR constant. We also use os.path.realpath to expand out any symlinks
  • This way, instead of storing the roots as i.e. ~/.local/share/Steam, they become i.e. /home/gaben/.local/share/Steam - Using a list comprehension just allowed me to do this without having to be repetitive with fstrings or os.path.join for every path in the list. I put the comprehension on multiple lines for readibility
  • os.path.realpath would turn a potential symlink, such as /home/gaben/.steam/root, into for example '/home/gaben/.local/share/Steam'.
  • The above changes essentially mean _POSSIBLE_STEAM_ROOTS might look something like this, assuming ~/.steam/root and ~/.steam/steam are symlinks to ~/.local/share/Steam (they are on my Arch PC and my Steam Deck):
    • Before: ['~/.local/share/Steam', '~/.steam/root', '~/.steam/steam', '~/.steam/debian-installation']
    • After: ['/home/gaben/.local/share/Steam', '/home/gaben/.local/share/Steam, '/home/gaben/.local/share/Steam', '/home/gaben/.steam/debian-installation']
  • Note how the paths are all expanded to use the full path instead of ~, and how the symlinked paths got expanded. This might result in duplicates, which we filter out.
• Filter out any duplicates we might end up with above (while preserving the order of the Steam root paths!) using list(dict.fromkeys(_POSSIBLE_STEAM_ROOTS)). This turns the values into the list into dictionary keys, and since dictionaries can't have duplicate keys, we filter out the duplicate paths.
  • This will transform our list from the example above as follows:
    • Before: ['/home/gaben/.local/share/Steam', '/home/gaben/.local/share/Steam, '/home/gaben/.local/share/Steam', '.steam/debian-installation']
    • After: ['/home/gaben/.local/share/Steam', '/home/gaben/.steam/debian-installation']
  • We don't use the list(set()) trick I described in my earlier comment because this does not preserve list order! I was heartbroken tbh, but StackOverflow saved the day.
  • This trick was picked up from this StackOverflow answer, which does note that this approach is slower than list(set()), but since we're only working with a handful of strings, I don't think this is a big deal for us.
• Build POSSIBLE_INSTALL_LOCATIONS first with a list of possible Steam install paths. We do this using a dictionary comprehension, setting the install_dir and vdf_dir values in the dictionary based on the _STEAM_ROOTs in _POSSIBLE_STEAM_ROOTS.
  • This also uses os.path.join to join the _STEAM_ROOT paths, just to ensure we always build a valid path.
  • We also only use a _STEAM_ROOT if it exists, so we don't end up adding paths that exist.
• Building POSSIBLE_INSTALL_LOCATIONS this way, combined with the new way we build _POSSIBLE_STEAM_ROOTS to store the realpath for a Steam root and filter out any duplicates, means our POSSIBLE_INSTALL_LOCATIONS at this point will only have a list of unique Steam install paths that actually exist. However, the installation paths may still be invalid, but we will now use util#is_valid_launcher_installation to check for a valid installation!
• Append the remaining launcher list of dictionaries with POSSIBLE_INSTALL_LOCATIONS += [...]`
  • I did consider making all of the paths use HOME_DIR instead of ~/, which would make those remaining paths consistent with how the Steam paths are now following this change, but I decided that that would be best left to a follow-up PR. So for now, all other launchers will still use ~, but in the near-future I'll make a PR to change this so they also use HOME_DIR.
  • There is a possibility for a future refactor where, we could only add paths to POSSIBLE_INSTALL_LOCATIONS if they actually exist. Since Steam now only has paths that actually exist, it could make sense to do this for the other launcher paths as well i.e. don't store ~/.var/app/net.lutris.Lutris/data/lutris/runners/wine/ if it doesn't exist. This might be trickier imo, as it is possible that the launcher will be installed, but the folder that we would install tools to does not exist. Probably unlikely and possibly this is an acceptable behaviour, but yeah, something for a different PR. Just a thought I had while working on this :-)

util.py Changes:

• In available_install_directories, only append install_dir to available_dirs if the path isn't already in the list.
• I didn't add any duplicate removal when we return available_dirs, but we could do the same kind of list(dict.fromkeys()) trick to remove any duplicate paths, although none should slip through.

The result of these changes in these two commits is that now we don't have the duplicated check for a valid Steam installation directory, and also that if a user has two (non-Flatpak, non-Snap) Steam installations, we can detect this automatically (a very niche use-case imo, but a possible benefit to come out of this!)

And the overall thing that this PR addresses is making sure that we only read from Steam installation directories that have the files we need (config.vdf and libraryfolders.vdf). The above two changes were mainly "cleanup" to avoid the duplicated check and make the logic a bit cleaner. Now constants.pydoesn't make any assumptions about whether a Steam root is valid, we just store ones that exist and then check if the actually exist when we check ouravailable_install_directories`. This also more closely matches what we do for other launchers.

I kept these changes across commits to hopefully make it a bit easier to follow the changes and the rationale behind them. All feedback welcome on this adjusted approach!

Something has went wrong somewhere and I pushed some bad changes, will fix...

It looks like 6423f52 is causing the problem. I narrowed the issue down to using os.path.join for the paths in POSSIBLE_INSTALL_LOCATIONS, as all of the other changes from that commit work except for that. Removing this os.path.join from both install_dir and vdf_dir fixes the problem.

For some reason, using os.path.join in this way gives this error: RecursionError: maximum recursion depth exceeded while calling a Python object

Hm, os.path.join isn't the issue. The issue appears even if I manually modify POSSIBLE_INSTALL_LOCATIONS to have the Steam path, like this:

POSSIBLE_INSTALL_LOCATIONS = [
    {'install_dir': '/home/emma/.local/share/Steam/compatibilitytools.d', 'display_name': 'Steam', 'launcher': 'steam', 'type': 'native', 'icon': 'steam', 'vdf_dir': '/home/emma/.local/share/Steam/config'},
    {'install_dir': '~/.var/app/com.valvesoftware.Steam/data/Steam/compatibilitytools.d/', 'display_name': 'Steam Flatpak', 'launcher': 'steam', 'type': 'flatpak', 'icon': 'steam', 'vdf_dir': '~/.var/app/com.valvesoftware.Steam/.local/share/Steam/config'},
    {'install_dir': '~/snap/steam/common/.steam/root/compatibilitytools.d/', 'display_name': 'Steam Snap', 'launcher': 'steam', 'type': 'snap', 'icon': 'steam', 'vdf_dir': '~/snap/steam/common/.steam/root/config'},
    {'install_dir': '~/.local/share/lutris/runners/wine/', 'display_name': 'Lutris', 'launcher': 'lutris', 'type': 'native', 'icon': 'lutris', 'config_dir': '~/.config/lutris'},
    {'install_dir': '~/.var/app/net.lutris.Lutris/data/lutris/runners/wine/', 'display_name': 'Lutris Flatpak', 'launcher': 'lutris', 'type': 'flatpak', 'icon': 'lutris', 'config_dir': '~/.var/app/net.lutris.Lutris/config/lutris'},
    {'install_dir': '~/.config/heroic/tools/wine/', 'display_name': 'Heroic Wine', 'launcher': 'heroicwine', 'type': 'native', 'icon': 'heroic'},
    {'install_dir': '~/.config/heroic/tools/proton/', 'display_name': 'Heroic Proton', 'launcher': 'heroicproton', 'type': 'native', 'icon': 'heroic'},
    {'install_dir': '~/.var/app/com.heroicgameslauncher.hgl/config/heroic/tools/wine/', 'display_name': 'Heroic Wine Flatpak', 'launcher': 'heroicwine', 'type': 'flatpak', 'icon': 'heroic'},
    {'install_dir': '~/.var/app/com.heroicgameslauncher.hgl/config/heroic/tools/proton/', 'display_name': 'Heroic Proton Flatpak', 'launcher': 'heroicproton', 'type': 'flatpak', 'icon': 'heroic'},
    {'install_dir': '~/.local/share/bottles/runners/', 'display_name': 'Bottles', 'launcher': 'bottles', 'type': 'native', 'icon': 'com.usebottles.bottles'},
    {'install_dir': '~/.var/app/com.usebottles.bottles/data/bottles/runners/', 'display_name': 'Bottles Flatpak', 'launcher': 'bottles', 'type': 'flatpak', 'icon': 'com.usebottles.bottles'}
]

Strange issue, I'll keep investigating...

d0e05ce seems to work, so it's something in 6423f52 causing the issue...

It seems like it is os.path.join causing the problems in 6423f52. I'll push a commit that removes that.

af05168 goes back to using f-strings for POSSIBLE_INSTALL_LOCATIONS, as using os.path.join was causing the recursion crash. Now things are working again.

I also removed the os.path.realpath call from the conditional in the POSSIBLE_INSTALL_LOCATIONS dictionary comprehensiion, as we do that in _POSSIBLE_STEAM_ROOTS, so we don't need to do it twice.

Noticed a couple of outdated and unclear comments, I will rework them a bit later

I think with ed709d9 I'm good. If there's anything outstanding here let me know and I'll try fix this up!

So now overall the changes in this PR are:

• Remove the VDF file check from constants.py
• Store all unique (no duplicates) Steam installation directories that actually exist on the filesystem in POSSIBLE_INSTALL_LOCATIONS, i.e. we will store ~/.local/share/Steam and ~/.steam/root if they both exist, and are not symlinks
• Use is_valid_launcher_installation to determine if the install paths in POSSIBLE_INSTALL_LOCATIONS are actually valid based on conditions specific to each launcher
  • Right now, we only check for (non-Flatpak and non-Snap) Steam, and we determine if it is valid by checking if it contains the VDF data files.
  • For example, ~/.local/share/Steam and ~/.steam/root might exist, but ~/.steam/root might be empty, or contain only empty directories, or otherwise it will be missing the VDF files we need. But it exists, so it's in POSSIBLE_INSTALL_LOCATIONS. Our function is_valid_launcher_installation will catch that even though the directory exists, it doesn't contain the VDF files we need, and thus ProtonUp-Qt will not use it, resolving the problem that occurs in Crash if non-flatpak steam is missing #353 - the empty directory structure existed but the VDF files are gone because Steam was uninstalled, but this PR now introduces checks to make sure the VDF files exist.
  • The reason our check for VDF files from before was not sufficient is because in constants.py we will always default to _POSSIBLE_STEAM_ROOTS[0] (which is ~/.local/share/Steam). So if in our loop we never found a directory with the VDF files we wanted, i.e. there was no valid Steam install found, we would still select ~/.local/share/Steam. The initial fix in this PR re-used the logic from constants.py and put it in a separate function, and the rest of the work in this PR was to refactor that duplicated logic out of constants.py so that we only do it in the one new function, is_valid_launcher_installation.

I wonder if it's worth extending our Steam check to determine if the VDF files are > 0 bytes, or some other magic-btye value, like 16 bytes? I dunno if it's too valuable. In most places we just check if a file exists, although there's no reason we couldn't create a general utility function to check if a file is valid, and as part of that check, include an optional parameter to check the size of a file. For example we could create a util function:

def is_file_valid(file_path: str, min_bytes: int = 0):
    return os.path.isfile(file_path) and os.path.getsize(file_path) >= min_bytes

There might already be a builtin way to do this or a better way, and introducing this across the codebase would probably be a multi-PR initiative, but if we want to do this kind of check, we could introduce the function here or in a follow-up PR :-)

Thanks for the elaborate explainaiton! Code looks good.

For some reason, using os.path.join in this way gives this error: RecursionError: maximum recursion depth exceeded while calling a Python object
It seems like it is os.path.join causing the problems in 6423f52. I'll push a commit that removes that.

Strange... issue says something about ConfigParser.

We don't use the list(set()) trick I described in my earlier comment because this does not preserve list order! I was heartbroken tbh, but StackOverflow saved the day.

Sadly Sets aren't the way to go in most cases. They are neither fast nor deterministic...
You solution list(dict.fromkeys(_POSSIBLE_STEAM_ROOTS)) seems quite good, I think I've seen this approach already in some Python code.

Right now, we only check for (non-Flatpak and non-Snap) Steam, and we determine if it is valid by checking if it contains the VDF data files.

I'm glad that with Flatpak/Snap they "introduced" some standard paths.

I wonder if it's worth extending our Steam check to determine if the VDF files are > 0 bytes, or some other magic-btye value, like 16 bytes?

I'm not sure how likely it is that the file exists but doesn't have any content. I wouldn't do the magic checks as they could change in the future and that would break the app.
Let's keep this idea in mind for a future in case a similar issue is reported by someone